Add NatGateway data source #1294

shadycuz · 2017-07-31T21:00:19Z

I never used go and I'm not familiar with the inter workings of terraform but I do need this data source to fix #1293 and #1238. I had issues with getting this to compile so I could not run tests (fat chance they would have passed). Looking for help on how to proceed.

Thanks,
Levi

levi@La-Tower:~/repos/terraform-provider-aws$ make build
==> Checking that code complies with gofmt requirements...
gofmt needs running on the following files:
./aws/data_source_aws_nat_gateway.go
./aws/data_source_aws_nat_gateway_test.go
You can use the command: `make fmt` to reformat code.
GNUmakefile:30: recipe for target 'fmtcheck' failed
make: *** [fmtcheck] Error 1
levi@La-Tower:~/repos/terraform-provider-aws$ make fmt
gofmt -w $(find . -name '*.go' |grep -v vendor)
levi@La-Tower:~/repos/terraform-provider-aws$ make build
==> Checking that code complies with gofmt requirements...
go install
main.go:4:2: cannot find package "github.com/hashicorp/terraform/plugin" in any of:
        /usr/local/go/src/github.com/hashicorp/terraform/plugin (from $GOROOT)
        /home/levi/work/src/github.com/hashicorp/terraform/plugin (from $GOPATH)
main.go:5:2: cannot find package "github.com/terraform-providers/terraform-provider-aws/aws" in any of:
        /usr/local/go/src/github.com/terraform-providers/terraform-provider-aws/aws (from $GOROOT)
        /home/levi/work/src/github.com/terraform-providers/terraform-provider-aws/aws (from $GOPATH)
GNUmakefile:7: recipe for target 'build' failed
make: *** [build] Error 1

I'm guessing I can build that dir tree and clone those repos into it?

shadycuz · 2017-07-31T21:33:32Z

Realized I will need to return these values...

                <item>
                    <networkInterfaceId>eni-00e37850</networkInterfaceId>
                    <publicIp>198.18.125.129</publicIp>
                    <allocationId>eipalloc-37fc1a52</allocationId>
                    <privateIp>10.0.2.147</privateIp>
                </item>

Is it against best practice to use your travis as a debugger?

shadycuz · 2017-07-31T22:29:27Z

This needs thoroughly tested and reviewed but we might have a working PR. Wish I could test on my local machine in Terraform. Instructions on how would be greatly appreciated.

shadycuz · 2017-07-31T22:32:51Z

I will update aws/provider.go and the website documentation tomorrow.

clintoncwolfe · 2017-08-01T02:45:38Z

Here's how I am locally testing.

Install go and set GOPATH, per golang.org
Per https://github.com/terraform-providers/terraform-provider-aws#developing-the-provider , I cloned the AWS provider under the GOPATH, from the terraform-providers repo.
In that local repo, I then added your repo as a remote, and pulled your branch.
I set my AWS... environment variables to an empty test account.
I ran the acceptance tests using

make testacc TEST=./aws TESTARGS="-run=TestAccDataSourceAwsNatGateway"

I am running terraform 0.9.latest; but I do also have terraform 0.10.0-dev (which is master). The provider docs say 0.10 is required. That hasn't bit me yet - other things have.

shadycuz · 2017-08-01T02:47:03Z

Any success? Do we have a working pr? I will work on it again tomorrow.

…

On Jul 31, 2017 10:45 PM, "Clinton Wolfe" ***@***.***> wrote: Here's how I am locally testing. 1. Install go and set GOPATH, per golang.org 2. Per https://github.com/terraform-providers/terraform-provider- aws#developing-the-provider , I cloned the AWS provider under the GOPATH, from the terraform-providers repo. 3. In that local repo, I then added your repo as a remote, and pulled your branch. 4. I set my AWS... environment variables to an empty test account. 5. I ran the acceptance tests using make testacc TEST=./aws TESTARGS="-run=TestAccDataSourceAwsNatGateway" I am running terraform 0.9.latest; but I do also have terraform 0.10.0-dev (which is master). The provider docs say 0.10 is required. That hasn't bit me yet - other things have. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1294 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQTJt_ZYzuELe7HwgcO3e8y6R8jLxcEUks5sTpFTgaJpZM4Oo6-G> .

clintoncwolfe · 2017-08-01T02:59:10Z

It's close - missing some pieces in the acctest config; a NGW resource requires a subnet, so that means a data lookup for a subnet; which likely means a data lookup for the default VPC... I may look around at other acctests and see how they handled that - not sure we should assume a default VPC is present, or just make what we need. Probably the latter.

Also I've added it to the data source list for the provider, but it's not picking it up. I imagine that is something silly. I'll put another hour or two into it tonight, then open a PR on your repo from my fork.

[clinton@Carol terraform-provider-aws]$ make testacc TEST=./aws TESTARGS="-run=TestAccDataSourceAwsNatGateway"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccDataSourceAwsNatGateway -timeout 120m
=== RUN   TestAccDataSourceAwsNatGateway
--- FAIL: TestAccDataSourceAwsNatGateway (1.36s)
	testing.go:428: Step 0 error: Configuration is invalid.

		Warnings: []string(nil)

		Errors: []string{"aws_nat_gateway.test: \"allocation_id\": required field is not set", "aws_nat_gateway.test: \"subnet_id\": required field is not set", "aws_nat_gateway.test: : invalid or unknown key: tags", "data.aws_nat_gateway.test_by_id: Provider doesn't support data source: aws_nat_gateway", "data.aws_nat_gateway.test_by_tags: Provider doesn't support data source: aws_nat_gateway"}
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	1.388s
make: *** [testacc] Error 1

shadycuz · 2017-08-01T03:00:09Z

Thanks =)

…

On Jul 31, 2017 10:59 PM, "Clinton Wolfe" ***@***.***> wrote: It's close - missing some pieces in the acctest config; a NGW resource requires a subnet, so that means a data lookup for a subnet; which likely means a data lookup for the default VPC... I may look around at other acctests and see how they handled that - not sure we should assume a default VPC is present, or just make what we need. Probably the latter. Also I've added it to the data source list for the provider, but it's not picking it up. I imagine that is something silly. I'll put another hour or two into it tonight, then open a PR on your repo from my fork. ***@***.*** terraform-provider-aws]$ make testacc TEST=./aws TESTARGS="-run=TestAccDataSourceAwsNatGateway" ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -run=TestAccDataSourceAwsNatGateway -timeout 120m === RUN TestAccDataSourceAwsNatGateway --- FAIL: TestAccDataSourceAwsNatGateway (1.36s) testing.go:428: Step 0 error: Configuration is invalid. Warnings: []string(nil) Errors: []string{"aws_nat_gateway.test: \"allocation_id\": required field is not set", "aws_nat_gateway.test: \"subnet_id\": required field is not set", "aws_nat_gateway.test: : invalid or unknown key: tags", "data.aws_nat_gateway.test_by_id: Provider doesn't support data source: aws_nat_gateway", "data.aws_nat_gateway.test_by_tags: Provider doesn't support data source: aws_nat_gateway"} FAIL exit status 1 FAIL github.com/terraform-providers/terraform-provider-aws/aws 1.388s make: *** [testacc] Error 1 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1294 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQTJtxljPPXz72ZHdGBWwc3wB4VZPM3Aks5sTpSAgaJpZM4Oo6-G> .

shadycuz · 2017-09-20T14:51:46Z

@Ninir @radeksimko When will this be reviewed by a team member and merged?

ewbankkit · 2017-09-24T22:29:21Z

You can now add "tags": tagsSchemaComputed() to the schema following merge of #1625.

ewbankkit · 2017-09-24T22:34:41Z

Adding back

			"network_interface_id": {
				Type:     schema.TypeString,
				Computed: true,
			},

would help with #1238 I think.

Ninir

Hey @shadycuz

This seems very good to me! 👍

$ make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAwsNatGateway'     
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccDataSourceAwsNatGateway -timeout 120m
=== RUN   TestAccDataSourceAwsNatGateway
--- PASS: TestAccDataSourceAwsNatGateway (212.44s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	212.472s

Just left a few cosmetic changes, and a request to add tags handling.

Thanks a lot for the work! 👍

Ninir · 2017-09-28T12:15:51Z

aws/data_source_aws_nat_gateway.go

+				Optional: true,
+				Computed: true,
+			},
+			"filter": ec2CustomFiltersSchema(),


Can you also add the tags part, following @ewbankkit 's comment?

Ninir · 2017-09-28T12:18:34Z

aws/data_source_aws_nat_gateway.go

+func dataSourceAwsNatGatewayRead(d *schema.ResourceData, meta interface{}) error {
+	conn := meta.(*AWSClient).ec2conn
+
+	log.Printf("[DEBUG] Reading NAT Gateways.")


Can you move this above line 85 and update it using something like:

log.Printf("[DEBUG] Reading NAT Gateway: %s", req)

This will print every parameters, for a better debugging :)

Ninir · 2017-09-28T12:21:20Z

website/aws.erb

@@ -140,6 +140,9 @@
                        <li<%= sidebar_current("docs-aws-datasource-kms-secret") %>>
                            <a href="/docs/providers/aws/d/kms_secret.html">aws_kms_secret</a>
                        </li>
+                        <li<%= sidebar_current("docs-aws-datasource-nat-gateway") %>>


Can you remove the extra plus signs on the lines below?

They have been removed

Ninir · 2017-09-28T12:22:26Z

website/docs/d/nat_gateway.html.markdown

+    Provides details about a specific Nat Gateway
+---
+
+# aws\_nat\_gateway


Antislashes are not needed anymore in titles, as per one of @sethvargo 's contributions :)

Ninir · 2017-09-28T12:22:47Z

website/docs/d/nat_gateway.html.markdown

+data "aws_nat_gateway" "default" {
+
+  subnet_id = "${var.subnet_id}"
+


Can you remove additional spaces also? will make it easier to read

Ninir · 2017-09-28T12:25:13Z

website/docs/d/nat_gateway.html.markdown

+Nat Gateway in the current region. The given filters must match exactly one
+Nat Gateway whose data will be exported as attributes.
+
+* `nat_gateway_id` - (Optional) The id of the specific Nat Gateway to retrieve.


You can name it id indeed

Ninir · 2017-09-28T12:25:44Z

website/docs/d/nat_gateway.html.markdown

+* `subnet_id` - (Optional) The id of subnet that the Nat Gateway resides in.
+
+* `vpc_id` - (Optional) The id of the VPC that the Nat Gateway resides in.
+


Can you remove additional spaces between list items? thanks 😄

shadycuz · 2017-10-04T00:55:30Z

@Ninir Waiting for time ;) maybe this weekend

Ninir · 2017-10-18T21:34:02Z

Hey @shadycuz

Do you think you can find time for that, or do you mind someone to take over this PR?

shadycuz · 2017-10-18T21:35:05Z

@Ninir I'll start pushing updates tonight

…form-provider-aws into feat/natgatewayDatasource

shadycuz · 2017-10-18T23:18:03Z

@Ninir Updated

shadycuz · 2017-10-18T23:19:05Z

@ewbankkit Can you explain your comment? #1294 (comment)
you linked back to my own issue ;) which is already fixed by this PR.

ewbankkit · 2017-10-19T11:57:49Z

@shadycuz I was commenting that the ENI ID/allocation ID of the NAT gateway should be outputs of the data source as that should address the linked-to issue.
I see that you have

allocated_eip_id
allocated_eni_id
allocated_private_ip
allocated_public_ip

as attributes but they're not declared in the data source's schema.Resource yet.
Also, do you think these attribute names should match the corresponding attribute names from the aws_nat_gateway resource?

allocation_id
network_interface_id
private_ip
public_ip

shadycuz · 2017-10-19T12:27:57Z

@ewbankkit for the attributes I export them because if you find a match they are available, however when searching for a nat gateway you are only allowed to use certain attributes. http://docs.aws.amazon.com/cli/latest/reference/ec2/describe-nat-gateways.html

I had thought about cheating, grabbing all natgateways in the aws account and then looping through them to find those attributes but after discussing it with someone else we decided to leave as is for now.

Yes I think the names should match what is in the resources, I will fix any that are different.

shadycuz · 2017-10-24T23:39:03Z

@ewbankkit updated attribute names to match the resource.

@Ninir Ready =)

Ninir

LGTM, thanks for the work @shadycuz! :)

$ make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAwsNatGateway'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccDataSourceAwsNatGateway -timeout 120m
=== RUN   TestAccDataSourceAwsNatGateway
--- PASS: TestAccDataSourceAwsNatGateway (216.63s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	216.664s

shadycuz · 2017-10-26T19:41:37Z

Thanks @Ninir , looking forward to seeing it in a release!

Thanks @clintoncwolfe

darend · 2017-11-07T21:15:21Z

aws/data_source_aws_nat_gateway.go

+
+	for _, address := range ngw.NatGatewayAddresses {
+		if *address.AllocationId != "" {
+			d.Set("allocation_id", address.AllocationId)


@Ninir I tried using this data source and found these attributes are not being included in the output. Running terraform with DEBUG logging I see this warning:

2017/11/07 13:04:19 [WARN] Output interpolation "private_ip" failed: Resource 'data.aws_nat_gateway.default' does not have attribute 'private_ip' for variable 'data.aws_nat_gateway.default.private_ip'

I do see the values in the AWS response logged by Terraform:

2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <DescribeNatGatewaysResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/"> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <requestId>1aa2f2a0-c316-4b83-808b-7402d5cfaa32</requestId> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <natGatewaySet> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <item> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <createTime>2017-11-03T23:34:36.000Z</createTime> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <natGatewayAddressSet> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <item> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <allocationId>eipalloc-xxxxxx</allocationId> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <networkInterfaceId>eni-xxxxxx</networkInterfaceId> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <privateIp>172.20.8.29</privateIp> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <publicIp>34.237.xyz.xyz</publicIp> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: </item> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: </natGatewayAddressSet> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <natGatewayId>nat-xxxxxxxx</natGatewayId> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <state>available</state> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <subnetId>subnet-xxxxxxxx</subnetId> 2017-11-07T13:04:17.147-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: <vpcId>vpc-xxxxxxxx</vpcId> 2017-11-07T13:04:17.148-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: </item> 2017-11-07T13:04:17.148-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: </natGatewaySet> 2017-11-07T13:04:17.148-0800 [DEBUG] plugin.terraform-provider-aws_v1.2.0_x4: </DescribeNatGatewaysResponse>

Does the list of output-able values have to be included in the Schema definition? https://github.com/terraform-providers/terraform-provider-aws/pull/1294/files#diff-5264f6e572425a4527111c5172455cf6R16

Opened #2209

ghost · 2020-04-10T17:40:36Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

shadycuz added 14 commits July 31, 2017 15:53

copied vpn_gateway to nat_gateway

be7aea2

replaced VpnGateway with NatGateway

457eaaf

replaced avalability_zone with subnet_id

c239530

replaced attached_vpc_id with vpc_id

28cd8e3

removed attachment.state and attachment.vpc references

a9b8ced

replaced s/VPN/NAT in logs/debugs/prints and s/vgw/ngw in var names

704cfcf

removed attachment loop

d9d565d

replaced s/VpnGateway/NatGateway

6026f3e

replaced s/vpn/nat

3740a02

removed references to attached/unattached

a5f9b28

ran go fmt

5a0f332

changed s/Filters/Filter

099fcd1

fixed returns, removed tags reference

57ad64d

fixed var names to be capital

37c2bd3

shadycuz added 4 commits July 31, 2017 17:36

removed unused import

07944dd

added loop to set address variables

9bba67a

Made NatGatewayAddresses plural

e58d0f0

changed empty string check

b6c7edb

Correct resource references in acctest config

64267b0

clintoncwolfe added 4 commits July 31, 2017 23:18

Add NGW to aws datasource list

c168d46

Add EIP, VPC, and Subnet to test fixture

c644192

Working test fixture config - added IGW, removed tagging on NGW

15f01f9

s/VPN/NAT/

1ee6a3f

Ninir reviewed Sep 28, 2017

View reviewed changes

Ninir added the waiting-response Maintainers are waiting on response from community or contributor. label Sep 28, 2017

shadycuz added 7 commits October 18, 2017 18:59

added tags to schema and moved log statement

8b5b0aa

removed plus signs

88de827

removed escape characters in title

baf3749

renamed id and removed extra spaces

7da3f4c

Merge branch 'master' into feat/natgatewayDatasource

e8e5fef

ran make fmt

eedf611

Merge branch 'feat/natgatewayDatasource' of github.com:shadycuz/terra…

fcbd7d9

…form-provider-aws into feat/natgatewayDatasource

made attributes and documentation similar to nat gateway resource

230e2b6

Ninir removed the waiting-response Maintainers are waiting on response from community or contributor. label Oct 26, 2017

Ninir approved these changes Oct 26, 2017

View reviewed changes

Ninir merged commit ab9e3e6 into hashicorp:master Oct 26, 2017

darend reviewed Nov 7, 2017

View reviewed changes

darend mentioned this pull request Nov 7, 2017

d/aws_nat_gateway: Add missing address attributes to the schema #2209

Merged

ewbankkit mentioned this pull request Jul 1, 2018

howto: find nat gateway for a given subnet? #1665

Closed

ghost locked and limited conversation to collaborators Apr 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NatGateway data source #1294

Add NatGateway data source #1294

shadycuz commented Jul 31, 2017

shadycuz commented Jul 31, 2017

shadycuz commented Jul 31, 2017

shadycuz commented Jul 31, 2017

clintoncwolfe commented Aug 1, 2017

shadycuz commented Aug 1, 2017 via email

clintoncwolfe commented Aug 1, 2017

shadycuz commented Aug 1, 2017 via email

shadycuz commented Sep 20, 2017

ewbankkit commented Sep 24, 2017

ewbankkit commented Sep 24, 2017

Ninir left a comment

Ninir Sep 28, 2017

Ninir Sep 28, 2017

Ninir Sep 28, 2017

shadycuz Oct 18, 2017

Ninir Sep 28, 2017

Ninir Sep 28, 2017

Ninir Sep 28, 2017

Ninir Sep 28, 2017

shadycuz commented Oct 4, 2017

Ninir commented Oct 18, 2017

shadycuz commented Oct 18, 2017

shadycuz commented Oct 18, 2017

shadycuz commented Oct 18, 2017

ewbankkit commented Oct 19, 2017

shadycuz commented Oct 19, 2017

shadycuz commented Oct 24, 2017

Ninir left a comment

shadycuz commented Oct 26, 2017 •

edited

Loading

darend Nov 7, 2017

darend Nov 7, 2017

ghost commented Apr 10, 2020

		data "aws_nat_gateway" "default" {

		subnet_id = "${var.subnet_id}"

		* `subnet_id` - (Optional) The id of subnet that the Nat Gateway resides in.

		* `vpc_id` - (Optional) The id of the VPC that the Nat Gateway resides in.

Add NatGateway data source #1294

Add NatGateway data source #1294

Conversation

shadycuz commented Jul 31, 2017

shadycuz commented Jul 31, 2017

shadycuz commented Jul 31, 2017

shadycuz commented Jul 31, 2017

clintoncwolfe commented Aug 1, 2017

shadycuz commented Aug 1, 2017 via email

clintoncwolfe commented Aug 1, 2017

shadycuz commented Aug 1, 2017 via email

shadycuz commented Sep 20, 2017

ewbankkit commented Sep 24, 2017

ewbankkit commented Sep 24, 2017

Ninir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shadycuz commented Oct 4, 2017

Ninir commented Oct 18, 2017

shadycuz commented Oct 18, 2017

shadycuz commented Oct 18, 2017

shadycuz commented Oct 18, 2017

ewbankkit commented Oct 19, 2017

shadycuz commented Oct 19, 2017

shadycuz commented Oct 24, 2017

Ninir left a comment

Choose a reason for hiding this comment

shadycuz commented Oct 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Apr 10, 2020

shadycuz commented Oct 26, 2017 •

edited

Loading